feat: add service-scoped env to Foundry extensions - #9079
Conversation
Co-authored-by: huimiu <107838226+huimiu@users.noreply.github.com>
Co-authored-by: huimiu <107838226+huimiu@users.noreply.github.com>
jongio
left a comment
There was a problem hiding this comment.
Seven threads from my earlier passes are still open on this HEAD with no replies. Listing them so they don't get lost now that the PR is showing approved.
env_refs.goresolveVarsdivergence: I added detail on that thread. The:-half fails silently, and a value the user was prompted for ends up in the Bicep param as literal text.helpers.gothe service env read swallows the RPC error and leaveshasServiceEnvironmentfalse, so the run path falls back to the full azd environment, which is the leak this PR closes. Routines and toolboxes return the error instead.agent_definition.goResolveAgentEnvironmentVariablefalls through to the callermappingfor any name a declaredenv:doesn't carry, and both callers hand it the full azd environment.resource_services.gosetServiceEnvironmentreturns before writing when collection comes back empty, so a service with no${VAR}references gets noenv:key and reads as legacy at run time.env_refs.goenvironmentReferenceAtnever checks that the byte after$is{."$HOME}"reportsOMEwhilefoundry.ExpandEnvleaves the value untouched, which is the exact invariantTestFindEnvironmentReferencesMatchesExpanderasserts.env_refs_test.goevery value in that parity corpus has$followed by{, so it can't catch the case above.env_refs_test.gotheTestNestedDefaultIsNotDiscoveredcomment says the outer name reaches init prompting, but the assertion four lines down isrequire.Empty.
None of these are new asks, they're the same threads. The two scanner ones and the two test ones are narrow. The three that fall back to the full azd environment are the ones I'd want settled before this merges, since they all undo the isolation the PR is adding.
trangevi
left a comment
There was a problem hiding this comment.
Approving, pending addressing Jon's comments, and with a possible suggestion to consider now or as a follow up, if it would indeed make things easier.
# Conflicts: # cli/azd/extensions/azure.ai.agents/README.md
jongio
left a comment
There was a problem hiding this comment.
One gap left over from the empty-env change.
init.go:2960andinit_from_code.go:873: agent init still goes through the guardedsetServiceEnvironment, so an agent with no collected variables gets noenv:key and falls back to the full azd environment at run time.
jongio
left a comment
There was a problem hiding this comment.
One issue with the empty-env fix in fc54476.
HIGH
setServiceEnvironmentScopesendsenv: {}over the config RPC, but core drops it before it reaches azure.yaml, so a generated agent service with no variables of its own still reads back as legacy and inherits the whole azd environment. Details inline onresource_services.go.
Both init call sites now route through the new function and the tests do pin that, so the gap isn't in this commit. It's one layer down, in how ServiceConfig.Environment is serialized.
jongio
left a comment
There was a problem hiding this comment.
Incremental change since my last review checks out. The unpersistable env: {} write is gone, both init paths and addResourceService route through setServiceEnvironment, and the comment records why an explicit empty env can't survive the save today. Build, vet, and the agents cmd suite pass locally against 88a6b76.
One optional follow-up: this behavior flipped twice in the PR, so a small test pinning that an empty environment writes no env: key would keep it from coming back by accident. resource_services_test.go is the natural home now that TestAddResourceServiceWritesEmptyEnvironment is gone.
jongio
left a comment
There was a problem hiding this comment.
Re-verified against 88a6b76: the shared ${VAR} scanner (escape collapsing so $${VAR} stays literal while $$${VAR} expands, balanced-default depth counting, and the probe-based check that keeps Foundry ${{...}} spans reserved), the Initialize reordering that resolves the azd environment before reading service environments, and the run-time precedence chain: process env, then service env, then the FOUNDRY_* platform values, then legacy definition env, with the Windows case-insensitive match preserved.
No blocking issues found.
Summary
Foundry extensions (agent, connection, routine, toolbox) now read service-scoped environment values from
ServiceConfig.Environmentinstead of the project-wide azd environment, and write generated variables into the standardservices.<name>.envblock.envkeep the old project-wide behavior.${VAR}scanners with one shared scanner.Follow-up to #8936. Fixes: #9231
Why this change
Service scoping. azd core now expands each service's environment before handing it to the extension. Extensions have to consume those values so services stop leaking variables into each other, while projects with no service-level
envkeep working unchanged.The empty-value bug.
azd upinitializes every service target while building the execution graph — before the user is prompted for a missing subscription or location. A config read at that moment holds empty strings, so the user answers the prompt and the extension still deploys a blank value, with no error anywhere. Two fixes: the agent target now uses the config passed to each deploy-time entrypoint instead of the copy cached atInitialize(re-running$refexpansion on it), and the provisioning provider resolves the azd environment before it reads service environments.One
${VAR}scanner. Writing anenv:block means deciding which variables a service references. #9212 answered a near-identical question — which variables to prompt for — with a second implementation, and the two had already drifted:env:block${A:-${B}}$count$They are now one scanner, pinned by a test to
foundry.ExpandEnv— the code that actually resolves these values — so it can't drift again. Escaping follows the expander too:$${VAR}is literal,$$${VAR}expands. The only remaining policy difference is one explicit line: init skips a reference that has a default, the env block records it.Nested references stay out of scope by design:
${OUTER:-${NESTED}}reportsOUTERonly.foundry.ExpandEnvstill resolvesNESTEDat deploy, but nothing discovers it, so it never reaches the generatedenv:block and init never prompts for it. That limitation is now stated in the scanner's doc comment and pinned by a test instead of left implicit — keep defaults literal.Environment terminology
envalso use them as the legacy fallback.envservices.<name>.env. Commands use the project config API to preserve them on disk.ServiceConfig.Environment. Consumed as-is, never expanded again.environmentVariablesor legacyagent.yaml. Serviceenvwins when both exist.Template syntax has distinct owners:
${VAR}is resolved by azd,${{...}}by Foundry, and$${{...}}preserves a Foundry expression through azd expansion.Manual Testing
azd ai agent init— migrated legacyenvironmentVariablestoenv, preserved${VAR:-default}, escaped Foundry templates as$${{...}}.azd ai agent run— serviceenvresolved for the child process while the template stayed intact on disk; legacy inline definitions resolved through the azd fallback.azd ai agent optimize apply— raw templates and scalars preserved through field-level updates.azd deploy— toolbox endpoint resolved from serviceenv, and from the azd fallback for a legacy toolbox.